Skip to content

docs: Add documentation for integrated vectorization. #834

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 6 commits into from
May 10, 2024
Merged

docs: Add documentation for integrated vectorization. #834

merged 6 commits into from
May 10, 2024

Conversation

superhindupur
Copy link
Contributor

Purpose

This PR adds a section to the README on Integrated Vectorization that links to a page that describes how to deploy it.
Required by #761.

Does this introduce a breaking change?

[ ] Yes
[x] No

Pull Request Type

What kind of change does this Pull Request introduce?

[ ] Bugfix
[ ] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[x] Documentation content changes
[ ] Other... Please describe:

How to Test

n/a

@github-actions GitHub Actions
Copy link

github-actions bot commented May 8, 2024 •
edited
Loading

Coverage

Coverage Report •
FileStmtsMissCoverMissing
TOTAL229767670% 
report-only-changed-files is enabled. No files were changed during this commit :)

Tests Skipped Failures Errors Time
178 0 💤 0 ❌ 0 🔥 11.723s ⏱️

adamdougal
adamdougal approved these changes May 8, 2024
View reviewed changes
Copy link
Collaborator

@adamdougal adamdougal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

ross-p-smith
ross-p-smith previously approved these changes May 8, 2024
View reviewed changes
adamdougal
adamdougal requested changes May 8, 2024
View reviewed changes
Copy link
Collaborator

@adamdougal adamdougal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed can we add a section about the inability to switch between push and pull.

@komalg1
Copy link
Collaborator

komalg1 commented May 8, 2024 •
edited
Loading

This is great. Just a couple of things we can add

  • we should tell users that they can select IV as true when they first deploy (Deploy to Azure button)
  • Use azd env set on a fresh deployment (when the index has not yet been created)
  • In case anyone wants to migrate they can delete the index in case of push & in case of pull delete index, indexer, datasource, skillset

Also, shall we hold on this merge till the time all IV changes are pushed in?

frtibble
frtibble reviewed May 8, 2024
View reviewed changes
@frtibble
Copy link
Contributor

frtibble commented May 8, 2024

Looks great! Once the IV changes are in, I'd be happy to go through the docs/steps to test it out as a user fresh to it.

gaurarpit
gaurarpit previously approved these changes May 8, 2024
View reviewed changes
Copy link
Contributor

@gaurarpit gaurarpit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

           (`-,-,
           ('(_,( )
            _   `_'
         __|_|__|_|_
       _|___________|__
      |o o o o o o o o/   
     ~'`~'`~'`~'`~'`~'`~

ross-p-smith
ross-p-smith reviewed May 8, 2024
View reviewed changes
@superhindupur superhindupur dismissed stale reviews from gaurarpit and ross-p-smith via 087ab7b May 9, 2024 09:11
@superhindupur @frtibble
Update README.md
087ab7b 
Co-authored-by: frtibble <32080479+frtibble@users.noreply.github.com>
@frtibble frtibble changed the title Add documentation for integrated vectorization. docs: Add documentation for integrated vectorization. May 9, 2024
@komalg1 komalg1 self-requested a review May 10, 2024 09:49
adamdougal
adamdougal approved these changes May 10, 2024
View reviewed changes
Copy link
Collaborator

@adamdougal adamdougal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@superhindupur superhindupur added this pull request to the merge queue May 10, 2024
Merged via the queue into main with commit a2f0312 May 10, 2024
@superhindupur superhindupur deleted the iv-documentation branch May 10, 2024 14:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ross-p-smith ross-p-smith ross-p-smith left review comments

@frtibble frtibble frtibble left review comments

@adamdougal adamdougal adamdougal approved these changes

@komalg1 komalg1 komalg1 approved these changes

@gaurarpit gaurarpit gaurarpit left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@superhindupur @komalg1 @frtibble @adamdougal @gaurarpit @ross-p-smith